aligned dataframe returns c("time", "model", "obs") but the convention in this module is c("model", "obvs", "time") and read by every metric. any reason align_by_time doesn't follow it ?

ur using model_path = ..., obs_path = ... but signature is run_benchmark(model_df, obs_df, ...) looks like README is from an earlier API where function took paths; folks using this ex would hit an error. probably want to update this and the parameter list ryt below it and pass as df

• i couldn't see load_and_map_data in NAMESPACE either a man page, looks like documen() hasn't re run
• wondering about registry format as we were discussing abt frictionless data / data package.json, was the simpler yaml approach an intentional phase 2 simplification with frictionless coming later or did direction shift ?
  either's fine, just want to make sure we're on same page, I'd defer to @dlebauer probably has perspective here

folks who passes df with column named timestamp or date instead of time hits df$time returning NULL, so error reads reads which is technically correct but doesn't tell user actual problem is missing column
worth checking column existence before type!

please use PEcAn.logger consistently, so messages land in the workflow log consistently

registry is in the ryt shape. but one thing, it's closed inside the fun body, so adding new metric means editing this function rather than registering one externally. worth adding to a package level object that callers can extend

bigger question, what's the plan for prediction interval coverage and PMU ? together they're CAR SEP regulatory pass criterion: bias <= PMU AND >=90% coverage. neither metric is in PEcAn.benchmark today, and neither's in this PR. they need different inputs than current dat$model / dat$obs (PMU needs per treatment pair SE and replicate counts; coverage needs per prediction quantiles)
worth thinking about whether the input should grow to support them now rather than reworking later

NSE / MEF is a third worth adding, not regulatory required, but standard in the broader literature for cross paper comparability.
cheap if you're already adding metrics

couple of gaps here. none of the tests exercise R2 path, so bug I flagged regarding that ships green.
so worth tightening at least the R2 path, the alignment value assertion, and tolerance dropping case before this lands

tolerance filter is silent here. model and obs typically run at different timesteps in validation work, so default tolerance can drop most model points without user knowing.
add one line info log of kept vs dropped would make this visible

observations are typically sparse relative to model output in validation workflows. plotting both as geom_line connects obs points with segments that imply information they don't have. convention is points for obs, line for model